Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Import Circle Game (used for testsuite) #177

Closed
wants to merge 5 commits into from

Conversation

jdetter
Copy link
Collaborator

@jdetter jdetter commented Oct 22, 2024

Description of Changes

Describe what has been changed, any new features or bug fixes

API

  • This is an API breaking change to the SDK

If the API is breaking, please state below what will break

Requires SpacetimeDB PRs

List any PRs here that are required for this SDK change to work

None

Testing

Write instructions for a test that you performed for this PR

  • dotnet testsuite still passes
  • testsuite builds, although there are no tests to run because those are in a separate PR (Unity Testsuite #176)

proof this builds:

image

@jdetter jdetter changed the base branch from master to staging October 22, 2024 09:49
@@ -75,3 +75,4 @@ obj~
# This is used for local paths to SpacetimeDB packages.
/nuget.config
/nuget.config.meta
.idea/
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated change: This ignores the jetbrains .idea/ directory which is created when opening the project in rider.

@jdetter jdetter marked this pull request as ready for review October 22, 2024 11:59
@jdetter jdetter requested review from RReverser, bfops and lcodes October 22, 2024 12:01
<ItemGroup>
<None Update="logo.png" Pack="true" PackagePath="" />
<None Update="README.md" Pack="true" PackagePath="" />
</ItemGroup>

<!-- Explicitly include all .cs files except Unity-related scripts -->
<ItemGroup>
<Compile Include="src/**/*.cs" Exclude="unity-tests/client/Assets/**/*.cs" />
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This prevents the unity testsuite cs files from being compiled during the dotnet testsuite.

.npmignore Outdated
@@ -0,0 +1 @@
unity-tests/
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this prevents the unity-tests directory from being downloaded when a user imports our SDK

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think .npmignore is respected for Git deps? IIUC Unity just uses Git clone, which doesn't have a way to exclude individual folders. We can only use the ~ suffix to exclude files and folders from assets, which you're already doing after the latest commits.

@cloutiertyler
Copy link
Collaborator

This comment also applies to this precursor PR: #176 (comment)

@cloutiertyler
Copy link
Collaborator

I am going to evaluate whether this should be a submodule prior to merging this.

@Centril Centril force-pushed the jdetter/unity-testsuite-import branch from a06a580 to e174841 Compare October 30, 2024 12:01
@Centril
Copy link
Contributor

Centril commented Oct 30, 2024

Rebased this atop of staging (trivial conflict to resolve, rc1 instead of 0.12.*)

@lcodes lcodes force-pushed the jdetter/unity-testsuite-import branch from e174841 to 3048408 Compare October 30, 2024 21:42
@jdetter
Copy link
Collaborator Author

jdetter commented Oct 31, 2024

closed in favor of using this as a submodule #186

@jdetter jdetter closed this Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants